Feat/home page#65
Conversation
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. WalkthroughThis PR establishes a modern React/TypeScript build and UI foundation while refactoring the Home page. It updates build configuration to include TypeScript compilation and Prettier formatting, adds React Query for server-state management, introduces shared layout and button components, and converts the Home page into a component-driven search interface with live GitHub suggestions and keyboard navigation. ChangesBuild tooling and configuration setup
UI foundation: utilities, hooks, components, and app shell
Home page search refactoring and new landing sections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Zahnentferner, again, there are too many insertions and deletions because I ran Prettier formatting on the files. Also, while switching branches, some line endings changed from CRLF to LF, which caused additional formatting changes. I’ve now added a |
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 18-19: The concurrency block was changed to use group:
github-pages and cancel-in-progress: true which may abort in-flight deployments
and leave the site in an inconsistent state; revert or adjust this by setting
the concurrency group back to the previous stable identifier (e.g., group:
pages) and set cancel-in-progress: false, or if you intentionally want
cancellations, add safeguards (e.g., only enable cancel-in-progress for
non-production branches) — update the concurrency settings in the deploy
workflow where the concurrency: group and cancel-in-progress values are defined.
In `@eslint.config.js`:
- Line 11: The ESLint config currently restricts linting via the files array
entry files: ["**/*.{ts,tsx}"] in eslint.config.js, so JS/JSX files are skipped;
update that files glob (the files property) to include JavaScript and JSX (for
example "**/*.{js,jsx,ts,tsx}" or a similar pattern that covers your src/**
tree) so existing src/*.js and src/*.jsx (e.g., App.jsx, hooks/useSortedData.js)
are linted by the React/React Hooks rules and JS-specific checks.
In `@package.json`:
- Line 3: The package.json "version" field was changed from 2.0.0 to 1.0.0 (a
major downgrade); either revert the package.json "version" property back to the
previous 2.0.0 value to preserve semantic versioning, or if the downgrade is
intentional, add a clear justification to the PR description and the commit
message explaining why the major version was decreased; update the "version"
property or documentation accordingly to resolve the review comment.
In `@src/components/Home/StatsSection.jsx`:
- Around line 15-17: The user-facing label in the StatsSection component
currently reads "Girth With PAT" (the <p> element with className "mt-2 text-sm
font-semibold uppercase text-white" in StatsSection.jsx) which is a
placeholder/typo; update that string to the intended copy (for example "Repos
With PAT" or the approved metric label) so the UI shows the correct
human-readable metric name.
In `@src/components/layout/Footer.tsx`:
- Around line 74-93: The footer currently renders client-side <Link> components
from footerLinks which point to /docs, /terms, /privacy, and /status but those
routes are not registered in your router (the catch-all Navigate redirects them
to "/"); update either the routing or the footer: either add corresponding Route
entries in your router (matching the paths and mapping to the new
Docs/Terms/Privacy/Status page components) or change the items in footerLinks
(or the rendering in Footer's Link map) to render external <a> anchors with
proper hrefs and target/rel attributes so they go to the real destinations;
locate the footerLinks array and the Link usage in Footer (the mapped Link
element) and modify accordingly.
In `@src/components/layout/Navbar.tsx`:
- Around line 34-41: In Navbar.tsx update the logo <img> for OrgExplorerLogo so
it no longer overflows the header and is loaded eagerly: replace the oversized
className "h-[10em]" with a height that fits the h-20 header (e.g., a Tailwind
height like "h-16" or "h-8"/"h-10" depending on desired padding) and change
loading="lazy" to loading="eager" (or remove the lazy attribute) on the <img>
element to ensure it's loaded above the fold.
- Line 23: The Tailwind variant used in the Navbar component's className is
using the incorrect "supports-backdrop-filter:…" form so the bg-black/80 rule
isn't generated; update the className on the Navbar (the JSX element that
currently contains className="... supports-backdrop-filter:bg-black/80 ...") to
use the Tailwind v4 feature-query bracketed variant by replacing the
supports-backdrop-filter token with the supports-[backdrop-filter] variant so
the backdrop-filter conditional background utility is emitted.
- Around line 71-75: The Button in Navbar.tsx is nesting a Link (anchor) inside
a real <button>, causing invalid interactive nesting; update the markup to
render the Link as the Button by using the Button's asChild prop (Radix Slot) so
the Link becomes the rendered element—replace the current Button/Link nesting
around FiSettings with a single Button with asChild and the Link
(to="/settings") as its direct child, keeping the FiSettings icon inside the
Link.
In `@src/components/RateLimitBanner.jsx`:
- Around line 32-37: In RateLimitBanner.jsx (RateLimitBanner component)
externalize all user-visible hardcoded strings — "API RATE LIMIT:", "REQUESTS
REMAINING", "Add PAT for 5,000 req/hr", and "RESETS HOURLY" — into your i18n
resource files and replace the literals with i18n lookups (e.g.,
t('apiRateLimit'), t('requestsRemaining'), t('addPat'), t('resetsHourly')) where
those strings appear (around the JSX that renders rateLimit.remaining/limit and
the PAT conditional). Ensure you import and use the project's i18n helper (e.g.,
useTranslation or t) and provide keys in the resource JSONs for each string,
updating any other occurrences at lines referenced (the strings at the other JSX
locations) to the same i18n keys.
In `@src/components/UI.jsx`:
- Around line 213-218: The component currently hardcodes user-visible strings
("Load More" button label and "Showing {shown} of {total}" text) inside the
render (button with onClick={onLoad} and the <p> showing shown/total); extract
these into i18n resource keys (e.g. load_more and showing_count) and replace the
literals with lookups from your app's i18n API (use the existing hook or helper
your project uses), passing shown and total as interpolation params for the
second string; add the new keys to the translation resource files for supported
locales and ensure the component imports/uses the translation function before
rendering (keeping the button props like C.btn("primary") and onLoad unchanged).
In `@src/components/ui/Button.tsx`:
- Around line 83-99: The Button currently sets Comp = asChild ? Slot : "button"
but always passes disabled={disabled || loading}; update the render so the
native disabled prop is only applied when Comp is the real "button" (i.e.,
asChild is false) and when asChild is true forward a11y-compatible semantics:
set aria-disabled={disabled || loading} on the Slot and add prevention of
activation (e.g., stop onClick/keyboard activation or add pointer-events-none /
tabIndex={-1} behavior) when disabled/loading; keep existing refs, className,
buttonVariants, and other props intact and ensure click/keyboard handlers are
suppressed for the Slot case so the control is non-interactive when disabled or
loading.
In `@src/context/AppContext.jsx`:
- Around line 48-50: Replace hardcoded user-visible error strings in
AppContext.jsx (the thrown Error message "No valid organizations found. Check
the names and try again." and the other messages around lines 89-93) with
references to your i18n resource keys and use the project's i18n helper (e.g.,
t('...') or i18n.translate) so messages are loaded from resource files;
create/choose appropriate keys like "errors.no_valid_organizations" and
"errors.github_rate_limit" in the locale files and update the Error construction
and any user-facing logging to use those keys instead of literal English text.
- Line 14: The PAT is being persisted insecurely via localStorage in the
AppContext component (state variables pat and setPat); replace localStorage
reads/writes with a secure storage mechanism—e.g., use expo-secure-store's
SecureStore.getItemAsync/SecureStore.setItemAsync (or migrate to HTTP-only
cookies handled by your backend) and update the initialization of pat and any
setPat calls to asynchronously load/save the token through SecureStore (or the
cookie API), removing direct localStorage usage to mitigate XSS token theft.
In `@src/pages/HomePage.jsx`:
- Around line 34-36: The fetch URL is interpolating debouncedQuery raw which
breaks on spaces/special chars; in HomePage.jsx before calling fetch (the
expression that builds
`https://api.github.com/search/users?q=${debouncedQuery}+type:org`) encode the
search portion with encodeURIComponent (for example encode the combined string
`${debouncedQuery} type:org` or separately encode debouncedQuery and append
`+type:org`) and use that encoded value in the q= param so the request is
well-formed; update the code that constructs the URL used by the fetch call to
reference the encoded query.
- Line 26: The JSON.parse call for const recent =
JSON.parse(localStorage.getItem("oe_recent") || "[]"); can throw on malformed
data; wrap this in a safe parse (e.g., try/catch or a small helper like
parseJsonSafe) so that on any parse error you return an empty array fallback and
avoid throwing during render; update the reference where recent is defined in
HomePage.jsx to use the safe parser (or fallback) instead of calling JSON.parse
directly.
- Around line 8-10: The imports in HomePage.jsx reference
"`@/components/home/`..." but the actual component folder is capitalized "Home",
causing failures on case-sensitive filesystems; update the import paths for
HeroSection, StatsSection, and OrgExplorerFeatures in src/pages/HomePage.jsx to
use "`@/components/Home/`..." (matching the directory name and each component
file) so the import specifiers exactly match the filesystem casing.
In `@src/styles/global.css`:
- Around line 65-69: Rename any camelCase keyframe names to kebab-case (e.g.,
change the `@keyframes` definition and all references from "fadeUp" to "fade-up");
update the `@keyframes` block named fadeUp and any CSS rules or JS that set
animation-name or animation: fadeUp to use "fade-up" instead, and ensure other
keyframe names (like "spin") already follow kebab-case or are renamed
consistently; search for the identifiers fadeUp and any usages in stylesheets,
components, or inline styles and update them to the new kebab-case name so
definitions and references match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f2eeba0a-f2ad-46b5-b787-3743e976d5b4
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (51)
.coderabbit.yaml.gitattributes.github/ISSUE_TEMPLATE/good_first_issue.yml.github/PULL_REQUEST_TEMPLATE.md.github/release-drafter.yml.github/workflows/deploy.yml.github/workflows/sync-pr-labels.yml.github/workflows/version-release.yml.pre-commit-config.yaml.prettierignore.prettierrcCONTRIBUTING.mdREADME.mdeslint.config.jspackage.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/Navbar.jsxsrc/components/RateLimitBanner.jsxsrc/components/UI.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/context/AppContext.jsxsrc/hooks/useDebounce.tssrc/hooks/useSortedData.jssrc/lib/utils.tssrc/main.jsxsrc/pages/AnalyticsPage.jsxsrc/pages/ContributorsPage.jsxsrc/pages/GovernancePage.jsxsrc/pages/HomePage.jsxsrc/pages/NetworkPage.jsxsrc/pages/OverviewPage.jsxsrc/pages/RepositoriesPage.jsxsrc/pages/SettingsPage.jsxsrc/services/analytics.jssrc/services/github.jssrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.jsvite.config.ts
💤 Files with no reviewable changes (2)
- vite.config.js
- src/components/Navbar.jsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/Home/OrgSearchBox.jsx (4)
1-7:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing "use client" directive for Next.js client component.
This component uses client-side features (event handlers:
onClick,onChange,onBlur,onFocus,onSubmit,onKeyDown) and must include the"use client"directive at the top of the file. As per coding guidelines, Next.js requires this directive for any component using client-side features.⚛️ Add "use client" directive
+"use client"; + import { FiSearch, FiX } from "react-icons/fi"; import { BsArrowRight } from "react-icons/bs";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 1 - 7, Add the Next.js client directive to the top of the OrgSearchBox component file so it runs as a client component; specifically insert "use client" as the very first line of src/components/Home/OrgSearchBox.jsx (the component that renders OrgSearchBox and imports/uses SearchSuggestions and attaches onClick, onChange, onBlur, onFocus, onSubmit, onKeyDown handlers) so Next.js recognizes the event-driven behavior.
56-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential duplicate chip and setTimeout memory leak.
Two issues in the
onBlurhandler:
Duplicate chip creation risk: Line 61 adds a chip on blur. However, from the parent's
handleKeylogic (context: HomePage.jsx), pressing Enter or comma also callsaddChip(input)but does not clear the input. This means typing text → pressing Enter → blurring will attempt to add the same chip twice (unlessaddChipinternally deduplicates).setTimeout not cleaned up: Lines 57-59 schedule
setShowSuggestions(false)without cleanup. If a suggestion is clicked,handleSelectOrgnavigates away immediately, unmounting the component while the timeout is still pending. This will trigger a "Can't perform a React state update on an unmounted component" warning.🔧 Recommended fixes
Fix 1: Clear input after adding chip to prevent duplicates
In the parent component's
handleKeyfunction, clear the input after adding a chip:if ((e.key === "Enter" || e.key === ",") && input.trim()) { e.preventDefault(); addChip(input); setInput(""); // Add this line }Fix 2: Clean up setTimeout
Convert onBlur to use useEffect or store timeout ID:
+const blurTimeoutRef = useRef(null); + +// ... in return: + onBlur={() => { - setTimeout(() => { + blurTimeoutRef.current = setTimeout(() => { setShowSuggestions(false); }, 200); input.trim() && addChip(input); }} + +// Add cleanup effect: +useEffect(() => { + return () => { + if (blurTimeoutRef.current) clearTimeout(blurTimeoutRef.current); + }; +}, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 56 - 62, The onBlur handler risks creating duplicate chips and leaking a pending timeout; update the flow so addChip duplicates are avoided and the timeout is cleaned up: ensure the caller (HomePage.handleKey) clears the input (call setInput("") after addChip) so pressing Enter/comma won't leave the same text to be added again on blur, and replace the inline setTimeout in OrgSearchBox.jsx (the onBlur that calls setShowSuggestions(false)) with a cancellable timer (store the timeout id in a ref and clearTimeout in a useEffect cleanup or clear it in onFocus/onClick handlers) so handleSelectOrg navigation/unmount doesn't trigger a state update on an unmounted component.
76-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExternalize button text for internationalization.
The "Explore" button text is hardcoded. As per coding guidelines, all user-visible strings should be externalized to i18n resource files.
🌐 Example i18n implementation
- Explore + {t('search.exploreButton')} <BsArrowRight className="size-4" />As per coding guidelines, user-visible strings must be externalized for internationalization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` at line 76, In OrgSearchBox.jsx replace the hardcoded "Explore" string in the JSX button element with a localized key (e.g. t('button.explore')) by importing/using your i18n hook (such as useTranslation from react-i18next) inside the OrgSearchBox component, call t('button.explore') where the "Explore" text currently appears, and add the corresponding "button.explore" entry to your i18n resource files (JSON) for each supported locale; ensure a sensible fallback is provided if the key is missing.
8-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNew component should be TypeScript (.tsx) with explicit prop types.
The PR description states TypeScript configuration was added and code was migrated from JavaScript to TypeScript. However, this new component file uses
.jsxextension with no type annotations for the 13 props. As per coding guidelines, explicit types should be used, and type imports should be preferred.🔷 Convert to TypeScript with prop interface
Rename file to
OrgSearchBox.tsxand add prop types:+interface OrgSearchBoxProps { + input: string; + setInput: (value: string) => void; + chips: string[]; + addChip: (chip: string) => void; + removeChip: (chip: string) => void; + handleKey: (e: React.KeyboardEvent<HTMLInputElement>) => void; + handleSubmit: (e: React.FormEvent<HTMLFormElement>) => void; + showSuggestions: boolean; + setShowSuggestions: (show: boolean) => void; + setSelectedIndex: (index: number) => void; + filteredSuggestions: Array<{ id: number; login: string; avatar_url: string }>; + selectedIndex: number; + isLoading: boolean; + handleSelectOrg: (org: string) => Promise<void>; +} + -export default function OrgSearchBox({ +export default function OrgSearchBox({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Home/OrgSearchBox.jsx` around lines 8 - 23, Rename OrgSearchBox.jsx to OrgSearchBox.tsx and add explicit TypeScript prop types by declaring an interface (e.g., OrgSearchBoxProps) that lists each prop (input, setInput, chips, addChip, removeChip, handleKey, handleSubmit, showSuggestions, setShowSuggestions, setSelectedIndex, filteredSuggestions, selectedIndex, isLoading, handleSelectOrg) with appropriate types (string, React.Dispatch/SetStateAction, arrays, functions with proper parameter/return types, boolean, number, etc.), import necessary React types (e.g., FC, Dispatch, SetStateAction) and use them on the component signature (export default function OrgSearchBox(props: OrgSearchBoxProps) or const OrgSearchBox: FC<OrgSearchBoxProps>), and update any implicit any callbacks/arrays (like filteredSuggestions and chips) to have concrete element types so the file compiles as TypeScript.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 53-54: Replace the hardcoded user-facing strings in the
OrgSearchBox component with i18n keys: import and use your i18n accessor (e.g.,
useTranslation().t) inside OrgSearchBox and replace the aria-label and
placeholder values with calls like t('orgSearch.ariaLabel') and
t('orgSearch.placeholder'); then add corresponding entries to the translation
resource files (for all supported locales) such as orgSearch.ariaLabel and
orgSearch.placeholder with the original English text and update any unit/story
tests that assert on these strings to use the i18n keys or translated values.
---
Outside diff comments:
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 1-7: Add the Next.js client directive to the top of the
OrgSearchBox component file so it runs as a client component; specifically
insert "use client" as the very first line of
src/components/Home/OrgSearchBox.jsx (the component that renders OrgSearchBox
and imports/uses SearchSuggestions and attaches onClick, onChange, onBlur,
onFocus, onSubmit, onKeyDown handlers) so Next.js recognizes the event-driven
behavior.
- Around line 56-62: The onBlur handler risks creating duplicate chips and
leaking a pending timeout; update the flow so addChip duplicates are avoided and
the timeout is cleaned up: ensure the caller (HomePage.handleKey) clears the
input (call setInput("") after addChip) so pressing Enter/comma won't leave the
same text to be added again on blur, and replace the inline setTimeout in
OrgSearchBox.jsx (the onBlur that calls setShowSuggestions(false)) with a
cancellable timer (store the timeout id in a ref and clearTimeout in a useEffect
cleanup or clear it in onFocus/onClick handlers) so handleSelectOrg
navigation/unmount doesn't trigger a state update on an unmounted component.
- Line 76: In OrgSearchBox.jsx replace the hardcoded "Explore" string in the JSX
button element with a localized key (e.g. t('button.explore')) by
importing/using your i18n hook (such as useTranslation from react-i18next)
inside the OrgSearchBox component, call t('button.explore') where the "Explore"
text currently appears, and add the corresponding "button.explore" entry to your
i18n resource files (JSON) for each supported locale; ensure a sensible fallback
is provided if the key is missing.
- Around line 8-23: Rename OrgSearchBox.jsx to OrgSearchBox.tsx and add explicit
TypeScript prop types by declaring an interface (e.g., OrgSearchBoxProps) that
lists each prop (input, setInput, chips, addChip, removeChip, handleKey,
handleSubmit, showSuggestions, setShowSuggestions, setSelectedIndex,
filteredSuggestions, selectedIndex, isLoading, handleSelectOrg) with appropriate
types (string, React.Dispatch/SetStateAction, arrays, functions with proper
parameter/return types, boolean, number, etc.), import necessary React types
(e.g., FC, Dispatch, SetStateAction) and use them on the component signature
(export default function OrgSearchBox(props: OrgSearchBoxProps) or const
OrgSearchBox: FC<OrgSearchBoxProps>), and update any implicit any
callbacks/arrays (like filteredSuggestions and chips) to have concrete element
types so the file compiles as TypeScript.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0de8331-50e1-4eb3-9750-a3cfe3256042
📒 Files selected for processing (1)
src/components/Home/OrgSearchBox.jsx
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
a68113a to
4b72098
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (4)
src/components/layout/Navbar.tsx (3)
71-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix invalid interactive nesting.
Nesting a
Link(anchor) inside aButton(button element) creates invalid HTML. TheButtoncomponent supports theasChildprop (Radix Slot) to render the Link as the underlying element.🛠️ Proposed fix
- <Button variant="ghost" size="icon" aria-label="Settings"> - <Link to={"/settings"}> + <Button asChild variant="ghost" size="icon" aria-label="Settings"> + <Link to="/settings"> <FiSettings /> </Link> </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 71 - 75, In Navbar.tsx fix the invalid interactive nesting by rendering the navigation Link as the Button's underlying element using the Button's asChild prop: replace the current Button wrapping a Link (Button > Link > FiSettings) with Button asChild so the Link becomes the actual rendered element and keep FiSettings as the child; update the JSX around Button, Link, and FiSettings accordingly to remove the nested anchor-in-button structure while preserving props like variant="ghost", size="icon", aria-label="Settings" and the to={"/settings"} on the Link.
34-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLogo dimensions and loading strategy need adjustment.
The logo height
h-[10em](~160px) overflows theh-20(80px) header. Additionally,loading="lazy"on an above-the-fold element can delay rendering and hurt LCP performance.🛠️ Proposed fix
<img src={OrgExplorerLogo} alt="OrgExplorer" - loading="lazy" + loading="eager" decoding="async" draggable={false} - className="h-[10em]" + className="h-16 w-auto" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` around lines 34 - 41, The logo image in Navbar.tsx uses className "h-[10em]" which overflows the header height (h-20) and also uses loading="lazy" which can harm above-the-fold LCP; update the <img src={OrgExplorerLogo} ... /> element to match the header by replacing "h-[10em]" with a constrained height (e.g., "h-20" or "max-h-20" / "h-auto" with an explicit max height) and remove loading="lazy" (or set it to "eager") so the logo loads promptly; ensure you keep decoding="async" and draggable={false}, and consider adding explicit width/height attributes or use intrinsic sizing to preserve aspect ratio.
23-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the correct Tailwind v4 syntax for feature queries.
supports-backdrop-filter:bg-black/80is invalid Tailwind v4 syntax. Feature queries must use the bracketedsupports-[...]form.🛠️ Proposed fix
- className="sticky top-0 z-50 w-full border-b-2 border-zinc-800 bg-black/95 backdrop-blur supports-backdrop-filter:bg-black/80" + className="sticky top-0 z-50 w-full border-b-2 border-zinc-800 bg-black/95 backdrop-blur supports-[backdrop-filter]:bg-black/80"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Navbar.tsx` at line 23, The Tailwind feature query token in the Navbar component's className is using the invalid `supports-backdrop-filter:bg-black/80`; update the className in Navbar.tsx (the JSX element with the className string shown) to use the bracketed feature query form, e.g. replace `supports-backdrop-filter:bg-black/80` with `supports-[backdrop-filter]:bg-black/80` so it conforms to Tailwind v4 syntax.src/components/layout/Footer.tsx (1)
74-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFooter navigation links point to unregistered routes.
The footer links
/docs,/terms,/privacy, and/statusare not defined insrc/App.jsx. The catch-all route will redirect them to/, resulting in broken navigation. Either add the corresponding routes or convert these to external links.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Footer.tsx` around lines 74 - 93, Footer links defined in footerLinks are using the SPA Link component but point to routes that aren't registered in the App component's route definitions (/docs, /terms, /privacy, /status), causing the catch-all to redirect to /. Fix by either (A) adding corresponding Route entries to the App component for those paths (create Docs/Terms/Privacy/Status pages and register them in the router), or (B) change the Footer.tsx Link usage to external anchors for those items (replace Link with <a href="..." target="_blank" rel="noopener noreferrer"> or set item.external flag and render <a> for external links) so they don't depend on client routes; update the footerLinks data structure or rendering logic in Footer.tsx to reflect the chosen approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Home/HeroSection.jsx`:
- Around line 24-30: The HeroSection component currently hardcodes user-visible
strings in the h1/span and p elements (“Architect Your”, “Insights”, and the
subtitle), which must be externalized to i18n resources; replace these literals
with calls to your i18n accessors (e.g., useTranslation hook or t('key')) inside
HeroSection so the h1 (and inner span), the paragraph, and the additional
strings referenced at lines 42-43 use translation keys (e.g., hero.title.part1,
hero.title.highlight, hero.subtitle) and load those keys from the locale
resource files, updating the locale JSON/YAML accordingly and ensuring any JSX
markup (span for highlighted text) still composes the translated pieces.
In `@src/components/Home/OrgExplorerFeatures.jsx`:
- Around line 5-11: The hardcoded user-facing strings in the OrgExplorerFeatures
component (the title, desc, and bullets object fields) must be moved into i18n
resource keys and referenced via the app's translation helper (e.g.,
useTranslation()/t) instead of inline literals; create keys like
"orgExplorer.features.interactive.title",
"orgExplorer.features.interactive.desc", and
"orgExplorer.features.interactive.bullets.[0..n]" in the locale files, import
and call t(...) inside OrgExplorerFeatures.jsx to populate title/desc/bullets,
and apply the same externalization pattern to the other hardcoded blocks noted
(lines 43-49, 79-85, 106-112, 136-138, 173-184) so all user-visible strings come
from the i18n resources.
- Around line 18-25: The render uses Math.random() inside the JSX map in the
OrgExplorerFeatures component which recalculates positions on every render and
causes jitter; fix it by computing a stable array of 8 position objects
(top/left) once and reusing them in the map (e.g., in OrgExplorerFeatures use
useMemo or useState initialized once to generate positions or a seeded RNG),
then replace the inline Math.random() expressions in the map with those
precomputed position values so the dots no longer change on each render.
In `@src/components/Home/OrgSearchBox.jsx`:
- Around line 56-62: The onBlur handler currently always calls addChip(input)
which creates unwanted chips during pointer-driven suggestion selection; update
the onBlur in the OrgSearchBox component to only hide suggestions (call
setShowSuggestions(false)) and remove the unconditional addChip call, and
instead add chips from the explicit user actions: handle Enter in the input's
onKeyDown to call addChip(trimmedInput) and call addChip from your suggestion
click handler; alternatively, if you must keep adding on blur, guard it by
inspecting the blur event's relatedTarget/nextActiveElement to ensure focus did
not move to a suggestion before calling addChip(input).
- Around line 40-44: Replace the clickable-only FiX icon with a proper
accessible button element so keyboard users can remove chips: wrap or replace
the FiX element with a <button> (or a Button component) that is focusable and
has type="button", an aria-label like "Remove chip" and uses the existing
onClick handler removeChip(c); ensure the button preserves the visual styling
(className "cursor-pointer opacity-70" or equivalent) and that any
stopPropagation/keyboard handling (Enter/Space) delegates to removeChip to
support activation via keyboard while keeping FiX as the visual child inside the
button.
In `@src/components/Home/QuickAccess.jsx`:
- Around line 6-8: The string "Quick Explore Access" in the JSX paragraph should
be pulled from i18n resources instead of a hard-coded literal; update the JSX in
the QuickAccess component (the <p className="text-xs uppercase tracking-[0.35em]
text-zinc-600"> element) to use the app's translation helper (e.g.,
t('quickAccess.heading') or intl.formatMessage({ id: 'quickAccess.heading' }))
and add the corresponding resource key "quickAccess.heading" with the translated
text to your i18n resource files; ensure you import or access the translation
function used across the project within QuickAccess.jsx.
In `@src/components/Home/RecentSearches.jsx`:
- Around line 10-12: The "Recent Searches" string in the RecentSearches.jsx
component is hardcoded; update the component to fetch this label from i18n
resources instead. Import and use your translation hook (e.g., useTranslation
from react-i18next) inside the RecentSearches component and replace the literal
string inside the span (the element with className "text-xs font-semibold
uppercase tracking-[0.3em] text-zinc-500") with the translated key (e.g.,
t('recentSearches') or the agreed resource key), and add the corresponding entry
to the translation resource files.
In `@src/components/Home/SearchSuggestions.jsx`:
- Around line 18-20: In SearchSuggestions.jsx replace the hardcoded user-visible
strings (e.g. the "Searching organizations..." div and the other two
suggestion/empty/loading labels in the same component) with i18n lookups: import
and call your translation helper (e.g. const { t } = useTranslation() from
react-i18next or the project's i18n utility), change the JSX to use
t('search.suggestions.searchingOrganizations') / t('search.suggestions.empty') /
t('search.suggestions.loading') (or equivalent keys), and add those keys and
translations to the locale resource files; ensure the component still renders
the same structure and fallbacks are provided if translation keys are missing.
In `@src/components/Home/StatsSection.jsx`:
- Around line 11-13: In the StatsSection.jsx component, replace all hardcoded
metric headings/descriptors (e.g., "Resource Load" and the other user-facing
strings inside the <p> elements in this component) with i18n keys and use your
app's translation helper (for example import and call the useTranslation hook/t
function) to render them (e.g., t('stats.resourceLoad')). Add corresponding keys
to the i18n resource files (e.g., stats.resourceLoad, stats.<otherKeys>) and
update the JSX in StatsSection (the <p> elements and any other metric labels) to
call t(...) instead of literal strings so all user-visible strings are
externalized.
In `@src/components/layout/Navbar.tsx`:
- Around line 95-135: The mobile nav is being mounted/unmounted with the
`{mobileMenuOpen && (...)}` conditional so its CSS transitions never run;
instead always render the <nav id="mobile-navigation"> and toggle its visibility
via classes driven by `mobileMenuOpen` (the same classnames currently used:
"max-h-96 opacity-100 translate-y-0" vs "max-h-0 opacity-0 -translate-y-2
pointer-events-none"), and also set an appropriate accessibility attribute
(e.g., aria-hidden={ !mobileMenuOpen }) so the element remains in the DOM for
the transitions inside the `nav` and the child list items rendered from
`navItems.map(...)` to animate correctly.
In `@src/components/ui/Button.tsx`:
- Around line 70-99: The button renders native <button> when asChild is false
and currently has no default type, causing accidental form submits; update the
render so that when Comp is the native "button" you pass a safe default type
that can still be overridden by callers (e.g., type={props.type ?? "button"} or
conditionally add { type: props.type ?? "button" } only when asChild is false)
and ensure this type prop is placed before spreading ...props so an explicit
props.type can override it; target the component rendering logic around Comp,
asChild, and the props spread in the Button component.
In `@src/pages/HomePage.jsx`:
- Line 12: quickExploreItems currently contains hardcoded user-visible labels;
move these labels into your i18n resource files (e.g., add keys like
quickExplore.aossieOrg, quickExplore.vercel, quickExplore.facebook,
quickExplore.microsoft) and replace the array of literal strings in HomePage.jsx
with an array of those keys or identifiers; then resolve them at render time
using the app's translation utility (e.g., useTranslation()/t or i18n.t) where
quickExploreItems is used so the UI displays translated values instead of
hardcoded strings.
- Around line 78-80: explore(orgs) currently swallows errors and sets AppContext
error state, so HomePage.jsx must not unconditionally call
navigate("/overview"); change the flow to only navigate when exploration
succeeded — either make explore (in AppContext.jsx) return a boolean/result
indicator and in HomePage.jsx do "const ok = await explore(orgs); if (ok)
navigate('/overview')" or, alternatively, read the AppContext error (e.g., via
useAppContext() after await explore(orgs)) and call navigate("/overview") only
when error is null/undefined; update the explore signature or the conditional in
HomePage.jsx accordingly so failed fetches do not trigger navigation.
In `@src/styles/global.css`:
- Around line 49-52: Remove the vendor-prefixed selection rule ::-moz-selection
and keep only the standard ::selection to satisfy the stylelint rule and avoid
duplication; locate the ::-moz-selection block in src/styles/global.css and
delete that selector and its declarations (or consolidate them under ::selection
if not already present) so only the modern ::selection styling remains.
---
Duplicate comments:
In `@src/components/layout/Footer.tsx`:
- Around line 74-93: Footer links defined in footerLinks are using the SPA Link
component but point to routes that aren't registered in the App component's
route definitions (/docs, /terms, /privacy, /status), causing the catch-all to
redirect to /. Fix by either (A) adding corresponding Route entries to the App
component for those paths (create Docs/Terms/Privacy/Status pages and register
them in the router), or (B) change the Footer.tsx Link usage to external anchors
for those items (replace Link with <a href="..." target="_blank" rel="noopener
noreferrer"> or set item.external flag and render <a> for external links) so
they don't depend on client routes; update the footerLinks data structure or
rendering logic in Footer.tsx to reflect the chosen approach.
In `@src/components/layout/Navbar.tsx`:
- Around line 71-75: In Navbar.tsx fix the invalid interactive nesting by
rendering the navigation Link as the Button's underlying element using the
Button's asChild prop: replace the current Button wrapping a Link (Button > Link
> FiSettings) with Button asChild so the Link becomes the actual rendered
element and keep FiSettings as the child; update the JSX around Button, Link,
and FiSettings accordingly to remove the nested anchor-in-button structure while
preserving props like variant="ghost", size="icon", aria-label="Settings" and
the to={"/settings"} on the Link.
- Around line 34-41: The logo image in Navbar.tsx uses className "h-[10em]"
which overflows the header height (h-20) and also uses loading="lazy" which can
harm above-the-fold LCP; update the <img src={OrgExplorerLogo} ... /> element to
match the header by replacing "h-[10em]" with a constrained height (e.g., "h-20"
or "max-h-20" / "h-auto" with an explicit max height) and remove loading="lazy"
(or set it to "eager") so the logo loads promptly; ensure you keep
decoding="async" and draggable={false}, and consider adding explicit
width/height attributes or use intrinsic sizing to preserve aspect ratio.
- Line 23: The Tailwind feature query token in the Navbar component's className
is using the invalid `supports-backdrop-filter:bg-black/80`; update the
className in Navbar.tsx (the JSX element with the className string shown) to use
the bracketed feature query form, e.g. replace
`supports-backdrop-filter:bg-black/80` with
`supports-[backdrop-filter]:bg-black/80` so it conforms to Tailwind v4 syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 25761c05-fb83-4938-be3a-a390f28cf481
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/logos/org-explorer-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
package.jsonsrc/App.jsxsrc/components/Home/HeroSection.jsxsrc/components/Home/OrgExplorerFeatures.jsxsrc/components/Home/OrgSearchBox.jsxsrc/components/Home/QuickAccess.jsxsrc/components/Home/RecentSearches.jsxsrc/components/Home/SearchSuggestions.jsxsrc/components/Home/StatsSection.jsxsrc/components/layout/Footer.tsxsrc/components/layout/Layout.tsxsrc/components/layout/Navbar.tsxsrc/components/ui/Button.tsxsrc/hooks/useDebounce.tssrc/lib/utils.tssrc/main.jsxsrc/pages/HomePage.jsxsrc/styles/global.csstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.ts
| Architect Your | ||
| <span className="block text-[#FCD34D]">Insights</span> | ||
| </h1> | ||
|
|
||
| <p className="mt-8 max-w-3xl text-base leading-relaxed text-zinc-400 sm:text-lg md:text-xl"> | ||
| Unified analytics across one or many GitHub organizations. | ||
| </p> |
There was a problem hiding this comment.
Externalize hero copy strings to i18n resources.
The heading, subtitle, and helper text are hardcoded user-visible strings in this component.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
Also applies to: 42-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/HeroSection.jsx` around lines 24 - 30, The HeroSection
component currently hardcodes user-visible strings in the h1/span and p elements
(“Architect Your”, “Insights”, and the subtitle), which must be externalized to
i18n resources; replace these literals with calls to your i18n accessors (e.g.,
useTranslation hook or t('key')) inside HeroSection so the h1 (and inner span),
the paragraph, and the additional strings referenced at lines 42-43 use
translation keys (e.g., hero.title.part1, hero.title.highlight, hero.subtitle)
and load those keys from the locale resource files, updating the locale
JSON/YAML accordingly and ensuring any JSX markup (span for highlighted text)
still composes the translated pieces.
Source: Coding guidelines
| title: "Interactive Org Visualization", | ||
| desc: "Explore repositories, teams, and contributors through a dynamic relationship graph designed for instant understanding.", | ||
| bullets: [ | ||
| "Live node graph", | ||
| "Repository mapping", | ||
| "Contributor insights", | ||
| ], |
There was a problem hiding this comment.
Externalize feature copy to i18n resources.
Feature titles/descriptions/bullets and section copy are hardcoded user-facing strings.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
Also applies to: 43-49, 79-85, 106-112, 136-138, 173-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgExplorerFeatures.jsx` around lines 5 - 11, The
hardcoded user-facing strings in the OrgExplorerFeatures component (the title,
desc, and bullets object fields) must be moved into i18n resource keys and
referenced via the app's translation helper (e.g., useTranslation()/t) instead
of inline literals; create keys like "orgExplorer.features.interactive.title",
"orgExplorer.features.interactive.desc", and
"orgExplorer.features.interactive.bullets.[0..n]" in the locale files, import
and call t(...) inside OrgExplorerFeatures.jsx to populate title/desc/bullets,
and apply the same externalization pattern to the other hardcoded blocks noted
(lines 43-49, 79-85, 106-112, 136-138, 173-184) so all user-visible strings come
from the i18n resources.
Source: Coding guidelines
| {[...Array(8)].map((_, i) => ( | ||
| <div | ||
| key={i} | ||
| className="absolute h-3 w-3 rounded-full bg-violet-400 shadow-[0_0_20px_rgba(167,139,250,0.8)]" | ||
| style={{ | ||
| top: `${20 + Math.random() * 60}%`, | ||
| left: `${15 + Math.random() * 70}%`, | ||
| }} |
There was a problem hiding this comment.
Avoid Math.random() inside render for node positioning.
Lines 23-24 regenerate positions every render, causing visual jitter and unnecessary layout churn.
♻️ Suggested stabilization
+import { useMemo } from "react";
+
export default function OrgExplorerFeatures() {
+ const visualizationNodePositions = useMemo(
+ () =>
+ Array.from({ length: 8 }, () => ({
+ top: `${20 + Math.random() * 60}%`,
+ left: `${15 + Math.random() * 70}%`,
+ })),
+ []
+ );
+
const features = [
@@
- {[...Array(8)].map((_, i) => (
+ {visualizationNodePositions.map((position, i) => (
<div
key={i}
className="absolute h-3 w-3 rounded-full bg-violet-400 shadow-[0_0_20px_rgba(167,139,250,0.8)]"
style={{
- top: `${20 + Math.random() * 60}%`,
- left: `${15 + Math.random() * 70}%`,
+ top: position.top,
+ left: position.left,
}}
/>
))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgExplorerFeatures.jsx` around lines 18 - 25, The render
uses Math.random() inside the JSX map in the OrgExplorerFeatures component which
recalculates positions on every render and causes jitter; fix it by computing a
stable array of 8 position objects (top/left) once and reusing them in the map
(e.g., in OrgExplorerFeatures use useMemo or useState initialized once to
generate positions or a seeded RNG), then replace the inline Math.random()
expressions in the map with those precomputed position values so the dots no
longer change on each render.
| <FiX | ||
| size={12} | ||
| className="cursor-pointer opacity-70" | ||
| onClick={() => removeChip(c)} | ||
| /> |
There was a problem hiding this comment.
Use a real button for chip removal (keyboard access).
Lines 40-44 attach removal to an icon click only. This is not keyboard-focusable and blocks accessible chip removal for non-pointer users.
♿ Suggested fix
- <FiX
- size={12}
- className="cursor-pointer opacity-70"
- onClick={() => removeChip(c)}
- />
+ <button
+ type="button"
+ onClick={() => removeChip(c)}
+ className="cursor-pointer opacity-70"
+ aria-label={`Remove ${c}`}
+ >
+ <FiX size={12} aria-hidden="true" />
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <FiX | |
| size={12} | |
| className="cursor-pointer opacity-70" | |
| onClick={() => removeChip(c)} | |
| /> | |
| <button | |
| type="button" | |
| onClick={() => removeChip(c)} | |
| className="cursor-pointer opacity-70" | |
| aria-label={`Remove ${c}`} | |
| > | |
| <FiX size={12} aria-hidden="true" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgSearchBox.jsx` around lines 40 - 44, Replace the
clickable-only FiX icon with a proper accessible button element so keyboard
users can remove chips: wrap or replace the FiX element with a <button> (or a
Button component) that is focusable and has type="button", an aria-label like
"Remove chip" and uses the existing onClick handler removeChip(c); ensure the
button preserves the visual styling (className "cursor-pointer opacity-70" or
equivalent) and that any stopPropagation/keyboard handling (Enter/Space)
delegates to removeChip to support activation via keyboard while keeping FiX as
the visual child inside the button.
| onBlur={() => { | ||
| setTimeout(() => { | ||
| setShowSuggestions(false); | ||
| }, 200); | ||
|
|
||
| input.trim() && addChip(input); | ||
| }} |
There was a problem hiding this comment.
onBlur should not auto-add chips unconditionally.
Lines 56-62 append input on every blur, which creates unintended chips during pointer-driven selection flows and other focus transitions.
🐛 Minimal fix
onBlur={() => {
setTimeout(() => {
setShowSuggestions(false);
}, 200);
-
- input.trim() && addChip(input);
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onBlur={() => { | |
| setTimeout(() => { | |
| setShowSuggestions(false); | |
| }, 200); | |
| input.trim() && addChip(input); | |
| }} | |
| onBlur={() => { | |
| setTimeout(() => { | |
| setShowSuggestions(false); | |
| }, 200); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Home/OrgSearchBox.jsx` around lines 56 - 62, The onBlur
handler currently always calls addChip(input) which creates unwanted chips
during pointer-driven suggestion selection; update the onBlur in the
OrgSearchBox component to only hide suggestions (call setShowSuggestions(false))
and remove the unconditional addChip call, and instead add chips from the
explicit user actions: handle Enter in the input's onKeyDown to call
addChip(trimmedInput) and call addChip from your suggestion click handler;
alternatively, if you must keep adding on blur, guard it by inspecting the blur
event's relatedTarget/nextActiveElement to ensure focus did not move to a
suggestion before calling addChip(input).
| {mobileMenuOpen && ( | ||
| <nav | ||
| id="mobile-navigation" | ||
| aria-label="Mobile Navigation" | ||
| className={cn( | ||
| "overflow-hidden text-center border-t-2 border-zinc-800 bg-black lg:hidden", | ||
| "transition-all duration-300 ease-in-out", | ||
| mobileMenuOpen | ||
| ? "max-h-96 opacity-100 translate-y-0" | ||
| : "max-h-0 opacity-0 -translate-y-2 pointer-events-none" | ||
| )} | ||
| > | ||
| <ul className="flex flex-col p-4"> | ||
| {navItems.map((item, index) => ( | ||
| <li | ||
| key={item.label} | ||
| className={cn( | ||
| "transition-all duration-300", | ||
| mobileMenuOpen | ||
| ? "translate-x-0 opacity-100" | ||
| : "-translate-x-4 opacity-0" | ||
| )} | ||
| style={{ | ||
| transitionDelay: `${index * 50}ms`, | ||
| }} | ||
| > | ||
| <Link | ||
| to={item.href} | ||
| className={cn( | ||
| "block rounded-md px-4 py-3 text-sm font-medium uppercase tracking-wide text-zinc-300 transition-colors", | ||
| "hover:bg-zinc-900 hover:text-[#FCD34D]", | ||
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#FCD34D] focus-visible:ring-offset-2 focus-visible:ring-offset-black" | ||
| )} | ||
| > | ||
| {item.label} | ||
| </Link> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </nav> | ||
| )} |
There was a problem hiding this comment.
Mobile menu transition won't animate due to conditional rendering.
The mobile menu is conditionally rendered with {mobileMenuOpen && (...)}, which mounts/unmounts the element. CSS transitions inside (lines 102-104) won't animate because the element is removed from the DOM immediately when closed. Keep the element mounted and use max-h-0 + opacity-0 + pointer-events-none to hide it while preserving transitions.
🛠️ Proposed fix
{/* MOBILE NAVIGATION */}
- {mobileMenuOpen && (
- <nav
+ <nav
id="mobile-navigation"
aria-label="Mobile Navigation"
className={cn(
"overflow-hidden text-center border-t-2 border-zinc-800 bg-black lg:hidden",
"transition-all duration-300 ease-in-out",
mobileMenuOpen
? "max-h-96 opacity-100 translate-y-0"
: "max-h-0 opacity-0 -translate-y-2 pointer-events-none"
)}
+ aria-hidden={!mobileMenuOpen}
>
<ul className="flex flex-col p-4">
{navItems.map((item, index) => (
<li
key={item.label}
className={cn(
"transition-all duration-300",
mobileMenuOpen
? "translate-x-0 opacity-100"
: "-translate-x-4 opacity-0"
)}
style={{
transitionDelay: `${index * 50}ms`,
}}
>
<Link
to={item.href}
className={cn(
"block rounded-md px-4 py-3 text-sm font-medium uppercase tracking-wide text-zinc-300 transition-colors",
"hover:bg-zinc-900 hover:text-[`#FCD34D`]",
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#FCD34D`] focus-visible:ring-offset-2 focus-visible:ring-offset-black"
)}
+ tabIndex={mobileMenuOpen ? 0 : -1}
>
{item.label}
</Link>
</li>
))}
</ul>
</nav>
- )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {mobileMenuOpen && ( | |
| <nav | |
| id="mobile-navigation" | |
| aria-label="Mobile Navigation" | |
| className={cn( | |
| "overflow-hidden text-center border-t-2 border-zinc-800 bg-black lg:hidden", | |
| "transition-all duration-300 ease-in-out", | |
| mobileMenuOpen | |
| ? "max-h-96 opacity-100 translate-y-0" | |
| : "max-h-0 opacity-0 -translate-y-2 pointer-events-none" | |
| )} | |
| > | |
| <ul className="flex flex-col p-4"> | |
| {navItems.map((item, index) => ( | |
| <li | |
| key={item.label} | |
| className={cn( | |
| "transition-all duration-300", | |
| mobileMenuOpen | |
| ? "translate-x-0 opacity-100" | |
| : "-translate-x-4 opacity-0" | |
| )} | |
| style={{ | |
| transitionDelay: `${index * 50}ms`, | |
| }} | |
| > | |
| <Link | |
| to={item.href} | |
| className={cn( | |
| "block rounded-md px-4 py-3 text-sm font-medium uppercase tracking-wide text-zinc-300 transition-colors", | |
| "hover:bg-zinc-900 hover:text-[#FCD34D]", | |
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#FCD34D] focus-visible:ring-offset-2 focus-visible:ring-offset-black" | |
| )} | |
| > | |
| {item.label} | |
| </Link> | |
| </li> | |
| ))} | |
| </ul> | |
| </nav> | |
| )} | |
| <nav | |
| id="mobile-navigation" | |
| aria-label="Mobile Navigation" | |
| className={cn( | |
| "overflow-hidden text-center border-t-2 border-zinc-800 bg-black lg:hidden", | |
| "transition-all duration-300 ease-in-out", | |
| mobileMenuOpen | |
| ? "max-h-96 opacity-100 translate-y-0" | |
| : "max-h-0 opacity-0 -translate-y-2 pointer-events-none" | |
| )} | |
| aria-hidden={!mobileMenuOpen} | |
| > | |
| <ul className="flex flex-col p-4"> | |
| {navItems.map((item, index) => ( | |
| <li | |
| key={item.label} | |
| className={cn( | |
| "transition-all duration-300", | |
| mobileMenuOpen | |
| ? "translate-x-0 opacity-100" | |
| : "-translate-x-4 opacity-0" | |
| )} | |
| style={{ | |
| transitionDelay: `${index * 50}ms`, | |
| }} | |
| > | |
| <Link | |
| to={item.href} | |
| className={cn( | |
| "block rounded-md px-4 py-3 text-sm font-medium uppercase tracking-wide text-zinc-300 transition-colors", | |
| "hover:bg-zinc-900 hover:text-[`#FCD34D`]", | |
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#FCD34D`] focus-visible:ring-offset-2 focus-visible:ring-offset-black" | |
| )} | |
| tabIndex={mobileMenuOpen ? 0 : -1} | |
| > | |
| {item.label} | |
| </Link> | |
| </li> | |
| ))} | |
| </ul> | |
| </nav> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/layout/Navbar.tsx` around lines 95 - 135, The mobile nav is
being mounted/unmounted with the `{mobileMenuOpen && (...)}` conditional so its
CSS transitions never run; instead always render the <nav
id="mobile-navigation"> and toggle its visibility via classes driven by
`mobileMenuOpen` (the same classnames currently used: "max-h-96 opacity-100
translate-y-0" vs "max-h-0 opacity-0 -translate-y-2 pointer-events-none"), and
also set an appropriate accessibility attribute (e.g., aria-hidden={
!mobileMenuOpen }) so the element remains in the DOM for the transitions inside
the `nav` and the child list items rendered from `navItems.map(...)` to animate
correctly.
| { | ||
| className, | ||
| variant, | ||
| size, | ||
| fullWidth, | ||
| loading, | ||
| asChild = false, | ||
| disabled, | ||
| children, | ||
| ...props | ||
| }, | ||
| ref | ||
| ) => { | ||
| const Comp = asChild ? Slot : "button"; | ||
|
|
||
| return ( | ||
| <Comp | ||
| ref={ref} | ||
| className={cn( | ||
| buttonVariants({ | ||
| variant, | ||
| size, | ||
| fullWidth, | ||
| loading, | ||
| }), | ||
| className | ||
| )} | ||
| disabled={disabled || loading} | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
Set a safe default type="button" for native button renders.
Without an explicit default, this component will submit enclosing forms unintentionally when used as a plain button.
♻️ Suggested fix
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
(
{
className,
variant,
size,
fullWidth,
loading,
asChild = false,
disabled,
+ type,
children,
...props
},
ref
) => {
const Comp = asChild ? Slot : "button";
return (
<Comp
ref={ref}
@@
- disabled={disabled || loading}
+ type={asChild ? undefined : type ?? "button"}
+ disabled={disabled || loading}
{...props}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/Button.tsx` around lines 70 - 99, The button renders native
<button> when asChild is false and currently has no default type, causing
accidental form submits; update the render so that when Comp is the native
"button" you pass a safe default type that can still be overridden by callers
(e.g., type={props.type ?? "button"} or conditionally add { type: props.type ??
"button" } only when asChild is false) and ensure this type prop is placed
before spreading ...props so an explicit props.type can override it; target the
component rendering logic around Comp, asChild, and the props spread in the
Button component.
| import StatsSection from "@/components/home/StatsSection"; | ||
| import OrgExplorerFeatures from "@/components/home/OrgExplorerFeatures"; | ||
|
|
||
| const quickExploreItems = ["AOSSIE-Org", "vercel", "facebook", "microsoft"]; |
There was a problem hiding this comment.
Externalize quick-access labels to i18n resources.
Line 12 hardcodes user-visible org labels in code. Move these to translation resources and resolve them at render time.
As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/HomePage.jsx` at line 12, quickExploreItems currently contains
hardcoded user-visible labels; move these labels into your i18n resource files
(e.g., add keys like quickExplore.aossieOrg, quickExplore.vercel,
quickExplore.facebook, quickExplore.microsoft) and replace the array of literal
strings in HomePage.jsx with an array of those keys or identifiers; then resolve
them at render time using the app's translation utility (e.g.,
useTranslation()/t or i18n.t) where quickExploreItems is used so the UI displays
translated values instead of hardcoded strings.
Source: Coding guidelines
| await explore(orgs); | ||
|
|
||
| const removeChip = c => setChips(prev => prev.filter(x => x !== c)) | ||
| navigate("/overview"); |
There was a problem hiding this comment.
Do not navigate to Overview when exploration fails.
On Line 80, navigation is unconditional. In src/context/AppContext.jsx, explore() catches failures and sets error state instead of throwing, so this flow still redirects after failed fetches.
🐛 Suggested contract fix
- await explore(orgs);
-
- navigate("/overview");
+ const ok = await explore(orgs); // make explore() return true/false (or throw)
+ if (ok) {
+ navigate("/overview");
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/HomePage.jsx` around lines 78 - 80, explore(orgs) currently
swallows errors and sets AppContext error state, so HomePage.jsx must not
unconditionally call navigate("/overview"); change the flow to only navigate
when exploration succeeded — either make explore (in AppContext.jsx) return a
boolean/result indicator and in HomePage.jsx do "const ok = await explore(orgs);
if (ok) navigate('/overview')" or, alternatively, read the AppContext error
(e.g., via useAppContext() after await explore(orgs)) and call
navigate("/overview") only when error is null/undefined; update the explore
signature or the conditional in HomePage.jsx accordingly so failed fetches do
not trigger navigation.
| ::-moz-selection { | ||
| background: rgba(252, 211, 77, 0.9); | ||
| color: #000000; | ||
| } |
There was a problem hiding this comment.
Remove the vendor-prefixed selection rule at Line 49.
::-moz-selection violates your current stylelint rule and duplicates ::selection behavior for modern browsers.
♻️ Suggested minimal fix
-::-moz-selection {
- background: rgba(252, 211, 77, 0.9);
- color: `#000000`;
-}As per coding guidelines: "**/*.css: Review the CSS code against the google css style guide and point out any mismatches."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ::-moz-selection { | |
| background: rgba(252, 211, 77, 0.9); | |
| color: #000000; | |
| } |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 49-49: Vendor-prefixed selector "::-moz-selection" (selector-no-vendor-prefix)
(selector-no-vendor-prefix)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles/global.css` around lines 49 - 52, Remove the vendor-prefixed
selection rule ::-moz-selection and keep only the standard ::selection to
satisfy the stylelint rule and avoid duplication; locate the ::-moz-selection
block in src/styles/global.css and delete that selector and its declarations (or
consolidate them under ::selection if not already present) so only the modern
::selection styling remains.
Sources: Coding guidelines, Linters/SAST tools
Screenshots/Recordings:
Before
https://1drv.ms/v/c/2b55451e453f4443/IQD4zvJn69ISQruWI4E14WXIAQljG07WmHb-TaKpKqNgpp4?e=c4ijFd
After
https://1drv.ms/v/c/2b55451e453f4443/IQByP7pqDHsET7G91kJmi_IxAfANKsKW7nRMgHpA7hfT-SE?e=4JtfEa
https://1drv.ms/v/c/2b55451e453f4443/IQBgpGkq-3QTSoKzBkoxJn_vAdySS0yXVy7QuPg1XKzmcGk?e=zGqXgc
Additional Notes:
Improved UI as per recommendation.
refactor: restructure Navbar and layout components
chore: set up TypeScript configuration
style: update global styles and integrate Tailwind CSS
fix: integrate React Query for data fetching
test: add utility functions and hooks
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores